The current jq query includes findings with ok severity in the SARIF output, mapping their level to none. This contradicts the documentation in docs/github-actions-sarif.md, which states: "ok → Not included in SARIF output (only findings)".

To align the implementation with the documentation and produce a cleaner SARIF file, you should filter out these ok findings entirely. This also allows for simplifying the level-mapping logic within the query.

          "results": [
            .[] | select(.severity != "ok") | {
              "ruleId": .id,
              "level": (
                if .severity == "critical" then "error"
                elif .severity == "warn" then "warning"
                else "note"
                end
              ),
              "message": {
                "text": .title,
                "markdown": (
                  if .remediation != "" then
                    (.description + "\n\n**Remediation:** " + .remediation)
                  else
                    .description
                  end
                )
              },
              "properties": {
                "evidence": .evidence,
                "auto_fix": .auto_fix
              }
            }
          ]

The SARIF_OUTPUT variable is used only within this main script to control output flow and does not need to be exported. Exporting variables that are not needed by child processes can pollute their environment and potentially lead to unexpected behavior if a child script uses a variable with the same name for a different purpose.

This condition [[ "$JSON_OUTPUT" -eq 0 ]] && [[ "$SARIF_OUTPUT" -eq 0 ]] && [[ "$QUIET" -eq 0 ]] is repeated 5 times in the script (here and on lines 159, 180, 205, 214). To improve readability and maintainability, consider refactoring this into a helper function or a single variable.

For example, you could define a variable after argument parsing:

# Determine if we should print the rich terminal UI
local should_print_ui=1
if [[ "$JSON_OUTPUT" -eq 1 || "$SARIF_OUTPUT" -eq 1 || "$QUIET" -eq 1 ]]; then
  should_print_ui=0
fi

Then you could replace all occurrences of the long condition with a simpler if [[ "$should_print_ui" -eq 1 ]]; then. This would make the code cleaner and easier to modify in the future.

This _sarif_level shell function is defined but never used. The mapping from the tool's severity levels to SARIF levels is handled directly inside the jq query on lines 106-112. This function is dead code and should be removed to keep the script clean.

SARIF results missing locations field - GitHub Code Scanning won't show inline file annotations

Current output only includes messages without file/line mappings. For GitHub to display findings inline on specific files, each result needs a locations array:

This maps all findings to the repository root. For more precise annotations, ClawPinch findings would need to include file paths (e.g., from $OPENCLAW_CONFIG evidence).

This is a comment left during a code review.
Path: scripts/helpers/sarif.sh
Line: 103:129

Comment:
SARIF results missing `locations` field - GitHub Code Scanning won't show inline file annotations

Current output only includes messages without file/line mappings. For GitHub to display findings inline on specific files, each result needs a `locations` array:

```suggestion
          "results": (
            . | map({
              "ruleId": .id,
              "level": (
                if .severity == "critical" then "error"
                elif .severity == "warn" then "warning"
                elif .severity == "info" then "note"
                elif .severity == "ok" then "none"
                else "note"
                end
              ),
              "message": {
                "text": .title,
                "markdown": (
                  if .remediation != "" then
                    (.description + "\n\n**Remediation:** " + .remediation)
                  else
                    .description
                  end
                )
              },
              "locations": [
                {
                  "physicalLocation": {
                    "artifactLocation": {
                      "uri": "."
                    }
                  }
                }
              ],
              "properties": {
                "evidence": .evidence,
                "auto_fix": .auto_fix
              }
            })
          )
```

This maps all findings to the repository root. For more precise annotations, ClawPinch findings would need to include file paths (e.g., from `$OPENCLAW_CONFIG` evidence).

How can I resolve this? If you propose a fix, please make it concise.

The generated helpUri points to the main repository page, which doesn't have anchors for check IDs, resulting in a broken link. It should point to the check-catalog.md file where the check details are documented.

To make this link work correctly, you would also need to add HTML anchors to the headers in references/check-catalog.md, for example: ### <a name="chk-cfg-001"></a>CHK-CFG-001 -- ....

This change updates the URI to point to the correct file. The anchor will work if you add the corresponding lowercase name attributes to the headers in the catalog.

The example helpUri here points to a non-existent anchor on the main repository page. This should be updated to reflect the correct link to the check documentation in references/check-catalog.md. This will prevent confusion for users looking at the example SARIF output and will align with the fix in sarif.sh.

Rules missing defaultConfiguration.level field

The SARIF spec and your own documentation (line 308-310 in docs/github-actions-sarif.md) show that rules should include defaultConfiguration.level, but this implementation doesn't include it. While not strictly required by SARIF v2.1.0, this field helps tools like GitHub Code Scanning understand the default severity of each rule.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

This is a comment left during a code review.
Path: scripts/helpers/sarif.sh
Line: 62:88

Comment:
Rules missing `defaultConfiguration.level` field

The SARIF spec and your own documentation (line 308-310 in `docs/github-actions-sarif.md`) show that rules should include `defaultConfiguration.level`, but this implementation doesn't include it. While not strictly required by SARIF v2.1.0, this field helps tools like GitHub Code Scanning understand the default severity of each rule.

```suggestion
              "rules": (
                . | [.[] | select(.severity != "ok")] | map({
                  id: .id,
                  name: .title,
                  shortDescription: {
                    text: .title
                  },
                  fullDescription: {
                    text: .description
                  },
                  helpUri: ($tool_uri + "#" + (.id | ascii_downcase)),
                  defaultConfiguration: {
                    level: (
                      if .severity == "critical" then "error"
                      elif .severity == "warn" then "warning"
                      elif .severity == "info" then "note"
                      else "note"
                      end
                    )
                  },
                  properties: {
                    category: (
                      if .id | startswith("CHK-CFG-") then "configuration"
                      elif .id | startswith("CHK-SEC-") then "secrets"
                      elif .id | startswith("CHK-NET-") then "network"
                      elif .id | startswith("CHK-SKL-") then "skills"
                      elif .id | startswith("CHK-PRM-") then "permissions"
                      elif .id | startswith("CHK-CRN-") then "cron"
                      elif .id | startswith("CHK-CVE-") then "cve"
                      elif .id | startsWith("CHK-SUP-") then "supply-chain"
                      else "general"
                      end
                    )
                  }
                })
                | unique_by(.id)
              )
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

The current jq expression to generate the helpUri anchor (.id | ascii_downcase) will result in broken links. GitHub's anchor generation for markdown headers is more complex (e.g., for ### CHK-CFG-001 -- ..., the anchor is #chk-cfg-001----...). Linking to the document itself is a more robust solution than generating a broken fragment link, allowing users to search for the check ID on the page.

The fallback version 1.2.0 is hardcoded here and is inconsistent with the version in package.json (1.2.1). This will cause an incorrect tool version to be reported in the SARIF file if reading package.json fails. Please update this to 1.2.1 for consistency.

To make this example more robust, consider using optional chaining (?) in the jq command. This prevents the step from failing if the SARIF file is valid but doesn't contain a results array (e.g., if no findings were produced), which is a good practice for CI workflows.

The tool version in this example SARIF output is 1.2.0, but the current version in package.json is 1.2.1. Please update the example to reflect the correct version for consistency.

To make this example more robust, consider using optional chaining (?) in the jq command. This will prevent the step from failing if the SARIF file is valid but contains no results array, for example, if no findings were produced.

This documentation states that findings will appear as inline comments. However, the current SARIF generation implementation in scripts/helpers/sarif.sh sets the location for all findings to the repository root ("uri": ".") without specific file or line number information. This means that GitHub will likely show findings at the repository level in the 'Security' tab, but not as inline annotations on the relevant source code files. To avoid user confusion, it would be best to update this documentation to reflect the current behavior. For true inline annotations, the tool's finding data model would need to be extended to include file and line information, which could be a larger future enhancement.

The tool version in this example is hardcoded as 1.2.1. The sarif.sh script dynamically determines the version from package.json. To prevent this documentation from becoming outdated, consider using a placeholder like <clawpinch-version> and mentioning that it's dynamically populated.

The example message object here doesn't quite match the structure generated by scripts/helpers/sarif.sh. The script generates a markdown message combining the finding's description and remediation fields, like (.description + "\n\n**Remediation:** " + .remediation). This example seems to use the title for the finding text. Aligning the example with the actual output would make the documentation more accurate.

SARIF includes null fields
properties.evidence and properties.auto_fix are emitted as JSON null when the input finding has those fields set to null (e.g., scanners using emit_finding ... "$evidence" with an empty/unset var typically produce empty string, but any custom scanner could output null). SARIF property values are expected to be strings/objects/arrays; null here is likely to break validation or downstream tooling. Consider normalizing with (.evidence // "") and (.auto_fix // "") (and similarly guarding .remediation comparisons against null) before constructing message.markdown and properties.

This is a comment left during a code review.
Path: scripts/helpers/sarif.sh
Line: 101:103

Comment:
**SARIF includes `null` fields**
`properties.evidence` and `properties.auto_fix` are emitted as JSON `null` when the input finding has those fields set to null (e.g., scanners using `emit_finding ... "$evidence"` with an empty/unset var typically produce empty string, but any custom scanner could output null). SARIF property values are expected to be strings/objects/arrays; `null` here is likely to break validation or downstream tooling. Consider normalizing with `(.evidence // "")` and `(.auto_fix // "")` (and similarly guarding `.remediation` comparisons against null) before constructing `message.markdown` and `properties`.


How can I resolve this? If you propose a fix, please make it concise.

Schema URL not stable
The $schema value in the example uses raw.githubusercontent.com/oasis-tcs/.../master/..., which can change over time and may cause validator drift. Since this doc is meant to be copied verbatim into CI troubleshooting, it should reference a specific tag/commit (or omit $schema entirely) to keep the guidance deterministic.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

This is a comment left during a code review.
Path: docs/github-actions-sarif.md
Line: 275:282

Comment:
**Schema URL not stable**
The `$schema` value in the example uses `raw.githubusercontent.com/oasis-tcs/.../master/...`, which can change over time and may cause validator drift. Since this doc is meant to be copied verbatim into CI troubleshooting, it should reference a specific tag/commit (or omit `$schema` entirely) to keep the guidance deterministic.


<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.